Skip to content
This repository was archived by the owner on Apr 6, 2023. It is now read-only.

docs: remove unnecessary return in example#3908

Closed
harmyderoman wants to merge 1 commit into
nuxt:mainfrom
harmyderoman:patch-1
Closed

docs: remove unnecessary return in example#3908
harmyderoman wants to merge 1 commit into
nuxt:mainfrom
harmyderoman:patch-1

Conversation

@harmyderoman
Copy link
Copy Markdown

@harmyderoman harmyderoman commented Mar 25, 2022

No need to return abortNavigation and navigateTo in defineNuxtRouteMiddleware

🔗 Linked issue

❓ Type of change

Docs

  • 📖 Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • 👌 Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

No need to return abortNavigation and navigateTo in defineNuxtRouteMiddleware

📝 Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

No need to return `abortNavigation` and `navigateTo` in `defineNuxtRouteMiddleware`
@pi0
Copy link
Copy Markdown
Member

pi0 commented Mar 25, 2022

Thanks for PR @harmyderoman. Indeed they are not needed but semantically making example readable that calling those functions causes rest of the logic to stop. Besides navigateTo is likely to return a promise which preserving chain is better than discarding.

@harmyderoman
Copy link
Copy Markdown
Author

harmyderoman commented Mar 25, 2022

@pi0 Every time I tried return navigateTo and got the error Uncaught (in promise) Error: Infinite redirect in navigation guard.
It works fine without return

@pi0
Copy link
Copy Markdown
Member

pi0 commented Mar 25, 2022

It shouldn't. I'll investigate.

@danielroe
Copy link
Copy Markdown
Member

Would you share a reproduction @harmyderoman? 🙏

@harmyderoman
Copy link
Copy Markdown
Author

harmyderoman commented Mar 26, 2022

Sorry guys, I did that infinity loop by trying to run return navigateTo(to.path). 😄
So stupid mistake.
It's just not so clear from the docs what this function returns and how to use it properly.

@pi0
Copy link
Copy Markdown
Member

pi0 commented Mar 26, 2022

No worries @harmyderoman and indeed thanks for sharing the cause. I think we might have improved redirectTo to avoid such loops.

@harmyderoman
Copy link
Copy Markdown
Author

Cool! So I'll close this PR.
Thank you for your great work!

@harmyderoman harmyderoman deleted the patch-1 branch March 26, 2022 16:21
@lixiaofa
Copy link
Copy Markdown

lixiaofa commented Dec 9, 2022

No worries @harmyderoman and indeed thanks for sharing the cause. I think we might have improved redirectTo to avoid such loops.

I also did that infinity loop by trying to run return navigateTo(to.path) .So stupid mistake.
image
image

What is the correct writing method?
@pi0 @harmyderoman

@harmyderoman
Copy link
Copy Markdown
Author

harmyderoman commented Dec 9, 2022

@lixiaofa if it's not fixed in new version, try it without return

@danielroe
Copy link
Copy Markdown
Member

You should not navigate to the current path as that will perform an extra navigation. Just return:

if (whitelist.includes(to.path)) {
  return
}

You can read more on https://router.vuejs.org/guide/advanced/navigation-guards.html#navigation-guards.

@lixiaofa
Copy link
Copy Markdown

@lixiaofa if it's not fixed in new version, try it without return

All permissions will be separated if they do not return.
Failed to achieve the effect of routing guard.

@lixiaofa
Copy link
Copy Markdown

You should not navigate to the current path as that will perform an extra navigation. Just return:

if (whitelist.includes(to.path)) {
  return
}

You can read more on https://router.vuejs.org/guide/advanced/navigation-guards.html#navigation-guards.

Thank you very much!

@danielroe danielroe added the 3.x label Jan 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants